Skip to content

bpo-30534: Fix error messages when pass keyword arguments#1901

Merged
serhiy-storchaka merged 5 commits into
python:masterfrom
serhiy-storchaka:call-no-keyword-args
Jun 6, 2017
Merged

bpo-30534: Fix error messages when pass keyword arguments#1901
serhiy-storchaka merged 5 commits into
python:masterfrom
serhiy-storchaka:call-no-keyword-args

Conversation

@serhiy-storchaka

Copy link
Copy Markdown
Member

to functions implemented in C that don't support this.

to functions implemented in C that don't support this.
@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Jun 1, 2017
@serhiy-storchaka serhiy-storchaka requested a review from vstinner June 1, 2017 07:38

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I also like included cleanup changes ;-)

On the issue, I requested unit tests. But I also proposed to change the error message, so we write tests later, when we agree on what should be done.

Comment thread Lib/test/test_call.py

def test_varargs0_kw(self):
self.assertRaises(TypeError, {}.__contains__, x=2)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may keep this test for other Python implementations. (This test doesn't check the error message.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate of the following test.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok, you are right.

Comment thread Lib/test/test_itertools.py Outdated
except TypeError as err:
# we expect type errors because of wrong argument count
self.assertNotIn("does not take keyword arguments", err.args[0])
print(repr(err))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove print()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread Objects/call.c Outdated
break;

case METH_VARARGS:
if (!(flags & METH_KEYWORDS) && nkwargs) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "!(flags & METH_KEYWORDS)" still needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed. Removed.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@serhiy-storchaka serhiy-storchaka merged commit 5eb788b into python:master Jun 6, 2017
@serhiy-storchaka serhiy-storchaka deleted the call-no-keyword-args branch June 6, 2017 15:45
@serhiy-storchaka

Copy link
Copy Markdown
Member Author

Thank you for your review @Haypo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants